Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implements ImportText() #1706

Closed
wants to merge 5 commits into from

Conversation

Zonespace27
Copy link
Contributor

@Zonespace27 Zonespace27 commented Mar 5, 2024

Implements the ImportText() proc.
Closes #1639

(Unchanged) Functionality

  • Can take multiple savefile insertions by adding ; to the 2nd argument
  • Can take a key-value pair by adding = in-between a string and a number/string in the 2nd argument
  • Can insert valueless entries
  • If you insert a key-value pair with an empty key (= 6), . will become equal to the value.

Parity Breaks

  • Changes the strange BYOND pseudo-errors into simple worldLogs
  • Requires there to be a space on both sides of an =. I have made this change because the overall functionality of the = was incredibly inconsistent, as shown below:
    • "foo = bar" is valid and fine
    • "foo= 1" causes an error (due to how ImportText() works, it will still insert foo\= into the savefile)
    • "foo =1" is valid and fine
    • "foo =bar" causes an error like bullet point 2

TODO

  • Write unit tests
  • Differentiate between having no value and having a null value

Shoutout to goon for being the only codebase I could find that uses this proc

@boring-cyborg boring-cyborg bot added the Runtime Involves the OpenDream server/runtime label Mar 5, 2024
@github-actions github-actions bot added the size/L label Mar 5, 2024
@boring-cyborg boring-cyborg bot added the Compiler Involves the OpenDream compiler label Mar 5, 2024
@Zonespace27 Zonespace27 marked this pull request as ready for review March 5, 2024 23:49
@amylizzle
Copy link
Collaborator

I would very much like to see some much more complex test cases here. It seems like you're only handling single line imports, which is only a small part of the functionality. It should be able to handle complex savefile structures, including objects.

This test should pass:

/obj/savetest
	var/obj/savetest/recurse = null

/proc/RunTest()
	var/obj/savetest/O = new() //create a test object
	O.name = "test"

	var/savefile/F = new() //create a temporary savefile

	F["dir"] = O
	F["dir2"] = "object(\".0\")"
	F["dir3"] = 1080
	F["dir4"] = "the afternoon of the 3rd"
	F["dir6/subdir6"] = 321
	F["dir7"] = null
	F["list"] << list("1",2,"three"=3,4, new /datum(), new /datum(), list(1,2,3, new /datum()))

	var/total_export = F.ExportText()

	var/savefile/F2 = new()
	F2.ImportText("/",total_export)
	//object test
	ASSERT(F2["dir"].name == "test")
	//string test
	ASSERT(F2["dir2"] == F["dir2"])
	ASSERT(F2["dir4"] == F["dir4"])
	//int test
	ASSERT(F2["dir3"] == F["dir3"])
	//null test
	ASSERT(F2["dir7"] == null)
	//subdir test
	ASSERT(F2["dir6"] == null)
	ASSERT(F2["dir6/subdir6"] == 321)
	//list test
	ASSERT(F2["list"][1] == "1")
	ASSERT(F2["list"][2] == 2)
	ASSERT(F2["list"]["three"] == 3)
	ASSERT(F2["list"][4] == 4)
	ASSERT(istype(F2["list"][5], /datum))
	ASSERT(istype(F2["list"][6], /datum))
	//nested list
	ASSERT(F2["list"][7][1] == 1)
	ASSERT(F2["list"][7][2] == 2)
	ASSERT(F2["list"][7][3] == 3)
	ASSERT(istype(F2["list"][7][4], /datum))

@Zonespace27
Copy link
Contributor Author

I would very much like to see some much more complex test cases here. It seems like you're only handling single line imports, which is only a small part of the functionality. It should be able to handle complex savefile structures, including objects.

Done, thank you for writing that!

@Zonespace27 Zonespace27 marked this pull request as draft March 6, 2024 23:27
@Zonespace27
Copy link
Contributor Author

Fairly sure I came about this the wrong way. I haven't had a lot of motivation recently, so I'm closing this for the time being. Might come back to it someday, but I don't want to squat on this proc's implementation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Compiler Involves the OpenDream compiler Runtime Involves the OpenDream server/runtime size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ImportText() is not implemented
2 participants